Skip to content

fix(petab): undo petab 0.10.0's log(10) factor on a converted log10 observable's sigma - #1017

Merged
wshlavacek merged 2 commits into
mainfrom
fix/petab-0.10
Nov 4, 2026
Merged

wshlavacek merged 2 commits into
mainfrom
fix/petab-0.10

Conversation

@wshlavacek

Copy link
Copy Markdown
Collaborator

CI started installing petab 0.10.0 (released 2026-10-26), and 10 tests in tests/test_petab_convert.py fail on every leg. One of the causes is a silently wrong objective, which this PR fixes in pybnf/petab/convert.py.

What petab 0.10.0 changed, and what it did to PyBNF

A v1 log10 observable now converts with a rescaled noise formula. petab 0.10.0 writes log10-normal as log-normal with noise formula log(10) * (<v1 formula>). That is exact in v2's own terms. petab1to2_preserve_scale states the scale differently (ADR-0073): it re-injects observableTransformation = log10 and resets noiseDistribution to normal. Under that column the v1 σ is the right one, so together the two gave a log10 residual whose σ was ln 10 times the stated one. For a v1 observable with noiseFormula = 0.1:

what PyBNF imports
petab 0.9.0 noise_model = lognormal, sigma = fix_at 0.1
petab 0.10.0, before this PR noise_model = lognormal, sigma = formula log(10) * (0.1), with no error or warning
petab 0.10.0, this PR noise_model = lognormal, sigma = fix_at 0.1

A placeholder σ failed at import instead, which is the failure CI showed.

The priors petab 0.9.x refused now convert. These are logNormal, logLaplace, and log10 parameterScaleUniform / parameterScaleLaplace. The converter already rewrites every declared prior from its v1 row (#893), so only the test that expected a refusal failed.

The warning text changed. petab's warning for the log10 case is reworded, so it was no longer silenced.

Commits

  1. fix(petab): undo petab 0.10.0's log(10) factor on a converted log10 observable's sigma.
    • restore_v1_noise_formulas puts each log10 observable's v1 noiseFormula back. It removes exactly the log(10) * (...) wrapper and leaves an unchanged formula alone. Any other rewrite is refused as a PybnfError naming both formulas, since the cell sets σ.
    • The new warning wording joins the silenced patterns.
    • On petab ≥ 0.10 the formerly refused priors run through the three existing prior oracles: hand-written v1/v2 densities, PyBNF's imported prior, and libpetab's own reading. That is what the test file's own comment said to do. The refusal test runs only where petab still refuses.
    • ADR-0073 gets a dated addendum.
  2. fix(petab): read the converted observables table as text. This fixes a review finding. On petab 0.9 with pandas < 3.1, a 17-digit numeric σ was refused, because the converter re-read the v2 table with a float parse that moves about 40% of such values by an ulp. The table is now read as text. Commit 2 also narrows my "same under 0.9 and 0.10" claim to the noise formula, and the test reads petab's version from package metadata.

How it was checked

  • Both petab releases: test_petab_convert.py plus the review file give 173 passed, 8 skipped on petab 0.10.0 / pandas 3.1.0, and 156 passed, 1 xfailed on petab 0.9.0 / pandas 3.0.5.
  • Mutation: with the restore disabled, both log10 full-conversion tests fail on 0.10, and 14 of the reviewer's 17 fail.
  • Independent oracle: the reviewer's tests compare the objective PyBNF builds from a converted, imported problem with libpetab's own v1 likelihood (calculate_llh) on the original tables. The cases cover numeric, placeholder, fixed-parameter, expression and prediction-dependent σ; log10, log and lin; and several observables across two files.
  • Siblings:
    • A log (natural) observable: petab never rescales it.
    • Native v2 problems don't go through petab1to2, and the PEtab export doesn't call it.
    • The reviewer ran a 69-case conversion matrix. Wherever both releases convert, the converted observables.tsv and the imported .conf are byte-identical across releases.
  • Independent review (a subagent that did not write the change) found nothing silently wrong in the change, and the one loud regression fixed in commit 2. Its tests are in tests/test_petab_convert_review.py.

Not in this PR (found in review)

  • petab 0.9's petab1to2 silently drops a pre-equilibration whose condition changes nothing. PyBNF then imports an experiment that starts from the initial conditions, which is a wrong objective, and pyproject still allows petab>=0.9. petab 0.10 keeps it. This is upstream and pre-existing; a strict xfail on petab 0.9 records it. PyBNF could guard against it, or raise its floor to petab 0.10.
  • A noise formula over non-estimated parameters fails at the first evaluation rather than at load: the job loop turns the error into a failed simulation.
  • The silenced "Parameter scales are not supported" warning now carries a petab 0.10 sentence about the maximum a posteriori estimate changing, for converted parameter-scale priors. ADR-0073's mapping is exactly that conversion; worth a look.

…bservable's sigma

petab 0.10.0 converts a v1 log10 observable to v2's natural-log family and
writes its noise formula as log(10) * (<v1 formula>), which is exact for that
family. petab1to2_preserve_scale states the scale differently (ADR-0073): it
re-injects observableTransformation = log10 and resets noiseDistribution to the
v1 linear base, and under that column the v1 sigma is the right one. Together
the two gave a log10 residual with sigma ln 10 times the stated one. A numeric
sigma imported as "sigma = formula log(10) * (0.1)" with no error (petab 0.9.0
imports "fix_at 0.1"); a placeholder sigma failed at import, which is the
failure main's CI showed.

restore_v1_noise_formulas puts each log10 observable's v1 noiseFormula back. It
removes exactly the log(10) * (...) wrapper, around the v1 formula as petab's
own v1 reader prints it, leaves an unchanged formula alone, and refuses any
other rewrite with both formulas named. petab 0.10.0's new warning for this
case joins the silenced patterns, since the converter repairs what it says.
The output is now the same under petab 0.9 and 0.10.

petab 0.10.0 also converts the v1 priors 0.9.x refused (logNormal, logLaplace,
log10 parameterScaleUniform / parameterScaleLaplace). The converter already
rewrites every declared prior from v1, so nothing changes there; on petab >=
0.10 those cases now run through the three existing prior oracles (hand-written
v1 and v2 densities, PyBNF's imported prior, libpetab's own reading), and the
refusal test runs only where petab still refuses.

Tests: tests/test_petab_convert.py passes on petab 0.10.0 (156 passed, 8
skipped) and 0.9.0 (140 passed). With the restore disabled, both log10
full-conversion tests fail on 0.10.0.

Signed-off-by: Bill Hlavacek <bill.hlavacek@gmail.com>
Independent review of the previous commit found a loud regression, reproduced
by running it: on petab 0.9 with pandas < 3.1, a log10 observable whose numeric
sigma has 17 significant digits was refused. The converter re-read the v2
observables table with pandas' default float parse, which moves about 40% of
17-digit values by an ulp, so the formula no longer matched the v1 text petab's
own reader printed, and restore_v1_noise_formulas refused it with a message
blaming petab1to2. The table is now read with dtype=str and
keep_default_na=False, so nothing in it is reparsed; the hint no longer tells
the user to switch petab release.

Also from the review: the claim that the output is the same under petab 0.9
and 0.10 is narrowed to the noise formula (log10 + laplace, and some
pre-equilibration shapes, still differ between the releases, loudly or as
described below), and the test reads petab's version from package metadata,
since petab.__version__ is deprecated.

The reviewer's tests are added (tests/test_petab_convert_review.py). They
check the objective PyBNF builds from a converted, imported problem against
libpetab's own v1 likelihood (calculate_llh) across numeric, placeholder,
parameter and prediction-dependent sigmas, several observables and two files,
and the refusal of an unknown rewrite through the whole converter. One strict
xfail remains, on petab 0.9 only: petab 0.9's petab1to2 drops a
pre-equilibration whose condition changes nothing, with no warning. That is
upstream and pre-existing, and is not addressed here.

Tests: test_petab_convert.py + the review file pass on petab 0.10.0 / pandas
3.1.0 (173 passed, 8 skipped) and petab 0.9.0 / pandas 3.0.5 (156 passed, 1
xfailed).

Signed-off-by: Bill Hlavacek <bill.hlavacek@gmail.com>
@wshlavacek
wshlavacek merged commit dd6f389 into main Nov 4, 2026
6 of 8 checks passed
@wshlavacek
wshlavacek deleted the fix/petab-0.10 branch November 4, 2026 18:36
wshlavacek added a commit that referenced this pull request Nov 4, 2026
Bump __version__ 1.8.1 -> 1.8.2, with CITATION.cff and docs/conf.py, and fold
the 108 staged fragments into CHANGELOG.md's [v1.8.2] section: 12 added, 10
changed, 1 removed, 85 fixed.

Raise the bngsim floor from 0.16.0 to 0.17.0 (pyproject dependencies, the
antimony extra, and the CI install in setup-pybnf), released together with this
version. The fixes the test suite credits to bngsim main rather than to 0.16.0
are all in a released build from 0.17.0 on, so an install resolved from PyPI
gets the answers those tests assert. The gradient fit's run-time gate still
names 0.16.0, the oldest build that code is right on. The two packaging tests
that pin the floor string follow it.

Built on main after #1017 (petab 0.10) and #1021 (bngsim 0.17.0's per-species
atol), which turned main's tests workflow green again.

Checked against bngsim 0.17.0 and petab 0.10.0 from PyPI:
test_packaging_metadata.py, test_sbml_solver_tolerances.py,
test_gradient_sens_fallback.py and test_petab_convert.py, 387 passed, 9
skipped; uv build and twine check --strict pass, and the wheel declares
Requires-Python >=3.12 and bngsim<1,>=0.17.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant